Skip to content

Conversation

AndreyYolkin
Copy link
Contributor

use registrar and single event listener to listen to keydown in a more performant way

@AndreyYolkin AndreyYolkin marked this pull request as draft July 2, 2025 18:21
@AndreyYolkin AndreyYolkin marked this pull request as ready for review July 2, 2025 18:30
}

if (getCurrentScope()) {
const stopListening = () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is your philosophy behind const functions and when to use them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used to write functions via const, but on top level it's converted by antfu eslint plugin automatically, unlike nesting 🥲

Copy link
Member

@johnleider johnleider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be reading this wrong, but I really thing you might benefit from using useRegistry instead of managing handlerMap.

Copy link

vercel bot commented Aug 31, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
vuetify0 Ready Ready Preview Comment Aug 31, 2025 3:51pm

Copy link

pkg-pr-new bot commented Aug 31, 2025

Open in StackBlitz

commit: 242bb48

function startGlobalListener () {
if (typeof document === 'undefined') return

if (!globalListener) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whenever if { ... } guards most of the logic within a function it should be replaced with early exit if (!..) return, so we avoid unnecessary indentation and the inner logic is easier to read/scan-through.

There are 3 methods here that would benefit from this quick refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants